Skip to content

fix(logging): harden sandbox log delivery under load - #3106

Closed
krishicks wants to merge 1 commit into
hicks/push-vlqrkupqqkrpfrom
hicks/push-vyrplousupmw
Closed

fix(logging): harden sandbox log delivery under load#3106
krishicks wants to merge 1 commit into
hicks/push-vlqrkupqqkrpfrom
hicks/push-vyrplousupmw

Conversation

@krishicks

@krishicks krishicks commented Sep 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Sandbox log delivery now recovers from large bursts and oversized records instead of becoming stuck retrying an undeliverable batch. Deliverable records continue reaching the gateway in order, and a single record that is too large no longer blocks the rest of the stream.

When load or connectivity makes loss unavoidable, gateway warnings and metrics now report the missing line count. That accounting remains accurate across stream reconnects and resets when the sandbox is deleted.

Related Issue

Refs #1055

Changes

  • Compress sandbox log pushes and split batches below the gateway decode limit so an oversized request cannot wedge the reconnect loop. Drop only individually undeliverable lines and report those losses to the gateway.
  • Track channel, reconnect-buffer, and oversized-line losses cumulatively. Preserve gateway accounting across RPC reconnects, reset it with sandbox teardown, and meter both sandbox-side and ingest-side drops.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@johntmyers

johntmyers commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Review findings

  • [P1] Do not require gzip before every gateway can accept it. The supervisor unconditionally enables request compression (log_push.rs:307-315), while gateway support is introduced only in this PR (multiplex.rs:273-275). A newer supervisor connecting to an older gateway or replica gets an unsupported-compression error, then enters the reconnect/backoff path and eventually drops logs. Please either land server-side acceptance before enabling client compression, negotiate the capability, or retry uncompressed when compression is rejected.

  • [P1] Requeue or account for batches rejected by the stream channel. send_logs returns false as soon as push_tx.send(...) fails, discarding the failed request and any remaining split groups (log_push.rs:233-257). Its callers have already removed those lines from batch with mem::take (log_push.rs:323-365), so an RPC closure during handoff silently loses them without rebuffering or incrementing the new drop counters. Please return the unsent groups for retry after reconnect, or explicitly record every discarded line before returning.

@krishicks

krishicks commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

@johntmyers

A newer supervisor connecting to an older gateway or replica gets an unsupported-compression error, then enters the reconnect/backoff path and eventually drops logs. Please either land server-side acceptance before enabling client compression, negotiate the capability, or retry uncompressed when compression is rejected.

Is this a real situation? A newer supervisor connecting to an older gateway? I would imagine only the reverse being true. During an upgrade the gateway would be bounced and the sandboxes shortly after with an updated supervisor, no?

@krishicks
krishicks force-pushed the hicks/push-vyrplousupmw branch from bb77402 to f6e9686 Compare September 2, 2026 20:17
@copy-pr-bot

copy-pr-bot Bot commented Sep 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@krishicks
krishicks force-pushed the hicks/push-vyrplousupmw branch 2 times, most recently from 2c3f99a to f572721 Compare September 2, 2026 23:32
@krishicks
fix(logging): keep sandbox logs flowing under load
278778d 
Sandbox log delivery now recovers from large bursts and oversized records
instead of becoming stuck retrying an undeliverable batch. Deliverable records
continue reaching the gateway in order, and a single record that is too large
no longer blocks the rest of the stream.

When load or connectivity makes loss unavoidable, gateway warnings and metrics
now report the missing line count. That accounting remains accurate across
stream reconnects and resets when the sandbox is deleted.

Refs #1055

Signed-off-by: Kris Hicks <khicks@nvidia.com>
@krishicks
krishicks force-pushed the hicks/push-vyrplousupmw branch from f572721 to 278778d Compare September 3, 2026 16:09
@krishicks

krishicks commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this in favor of a different split redone by gpt-6-astra.

@krishicks krishicks closed this Sep 9, 2026
@krishicks
krishicks deleted the hicks/push-vyrplousupmw branch September 9, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrunalp mrunalp Awaiting requested review from mrunalp
@sjenning sjenning Awaiting requested review from sjenning
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@krishicks @johntmyers